Skip to content

Rails 6.1 support and bit type bug fix #854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

Rails 6.1 support and bit type bug fix #854

wants to merge 10 commits into from

Conversation

@Circuit8
Copy link

@Circuit8 Circuit8 commented Feb 22, 2021 •
edited
Loading

This expands on #853 by fixing the test suite.

We also discovered that if a column is a "bit" type, and it has a default value of true, we would get an error from active record, as it seems that in the current version it is expecting boolean columns to be cast to "1" for true values, rather than true. This PR fixes that aswell.

@Circuit8 Circuit8 mentioned this pull request Feb 23, 2021
Closed
@marciojg
Copy link

marciojg commented Mar 2, 2021

@wpolicarpo Can you help us doing a merge of this PR and create a new release?

@joshleblanc joshleblanc mentioned this pull request Mar 22, 2021
Merged
@wpolicarpo
Copy link
Member

wpolicarpo commented Apr 13, 2021

@Circuit8 can you please rebase your branch against main?

@Circuit8
Copy link
Author

Circuit8 commented Apr 14, 2021

@wpolicarpo sure thing, should be sorted now.

@runephilosof-abtion
Copy link
Contributor

runephilosof-abtion commented Apr 15, 2021

@Circuit8 You did not rebase this branch onto main. You merged main into this branch.

From #861 (comment)

Not sure if that was the best approach to solve that issue. The fact that you had to change a spec rings a bell that something might be wrong there.

TinyTDS casts the bit data type to boolean, can't see why we should change it at the adapter level. That could lead to unexpected bugs/changes to already existing apps assuming boolean as true/false instead of 1/0.

In any case #875 addressed the original issue already.

@wpolicarpo
Copy link
Member

wpolicarpo commented Apr 15, 2021

The remaining changes in this PR were superseded by #875 so I'm closing this off.

@wpolicarpo wpolicarpo closed this Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

rails-6.1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Circuit8 @marciojg @wpolicarpo @runephilosof-abtion @lk0001